Skip to content

Conversation

Zacholme7
Copy link
Member

@Zacholme7 Zacholme7 commented Jul 8, 2025

Implementation for the types spec tests. Gave myself a week for these and think it may be time to just clean this all up and call them good. This PR contains the work in #374 and #377 as everything got a bit our of line so I decided to just merge everything together. This is a giga PR so I can also try to break it up.

I would say the main use for these is to test our SSZ encoding/decoding, which we can now confirm is spec compliant. There really may even be an argument that we should only include the ssz encoding/decoding and consider what we can scrap.

I have also deemed a few to be skippable

  • The committee member tests as our types differ substantially here and it would be a couple hundred lines of just parsing for basic quorum validation. I can just write a unit tests if this is really needed
  • The beacon deposit data test. This generates ETH deposit data, but this is never actually even used in the client.
  • The duty tests. These are for their internal runner mapping which is an implementation detail, not spec level.
  • The share encoding test. We use a different structure for share data so it does not make sense to test this encoding.
  • The max message size test require significant polymorphic parsing infrastructure and constructor mocking to run. Just manually verified the size which matches up to the spec

There are also some tests with data issues that I have raised
- The ssv message tests use message ids that should not be considered valid for the testing scenario. I have addresses this here

  • the ssz test uses data from a beacon block with mocked signatures which when we try to parse into BeaconBlock, fails with BLST_BAD_ENCODING. Raised the issue here

@Zacholme7
Copy link
Member Author

Currently blocked on #416 getting merged in

@Zacholme7 Zacholme7 changed the title Implement type spec tests feat(spec-tests): implement type spec tests Jul 21, 2025
@Zacholme7 Zacholme7 changed the base branch from unstable to spec-tests July 28, 2025 12:48
Zacholme7 and others added 12 commits July 28, 2025 14:28
…igp#448)

We have many hardcoded paths all over the codebase.


  Move all of them and directory creation to a `data_dir` module in global config.

There we can also soon add a lockfile mechanism!
…atabase switchups (sigp#387)

- closes sigp#357


  Introduce the `metadata` table where we track the currently used database schema. This will later allow us to implement "migrations", i.e. updating the database schema without requiring the user to resync.

Furthermore, when we create the database, we store the domain type used in order to avoid working with a database that exists, but was previously used on another network.

The existing `block` table is removed and it's column moved to `metadata` in order to have only one table with data that exists only once.
dknopik and others added 8 commits August 8, 2025 11:39
- sigp#471

This was caused by a bug in `multi_index_map`, see also lun3x/multi_index_map#65.


  Due to time pressure, revert the refactor sigp#463 for now for a smooth `v0.3.0` (and by extension `v1.0.0`) release.
This adds a CLAUDE.md file that provides guidance to Claude Code when working with this repository. The file includes:
- Common commands for building, testing, and linting
- Architecture overview
- Code organization details
- Development tips

🤖 Generated with [Claude Code](https://claude.ai/code)
sigp#437 introduced a bug, where anchor fails to collect signatures for committees with a sync duty.


  This reverts sigp#437.

Also change the incorrect comment, in order to prevent this from being removed again. Sorry!
)

I noticed a fun bug: When in a single batch (for historic sync) or block (for live sync) a validator is removed and readded again, the validator store does not notice the change. This is bad, as the private key share likely has changed. So we continue signing stuff with the old partial share, while correct nodes sign with the new partial share. This causes signatures to be invalid when recombined.


  First, I was tempted to fix this while maintaining the current structure, but I realized that iterating over the whole database every time and comparing it to everything in the validator store is quite nonsensical if we can use the database directly instead.

So this PR gets rid of the `DashMap`s in the validator store and reads needed info directly from the database state.
@Zacholme7 Zacholme7 closed this Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants